Add support for reading PDBs from AF2 database by a-r-j · Pull Request #102 · BioPandas/biopandas

a-r-j · 2022-04-30T14:46:25Z

Description

Simple PR. Adds an equivalent method (fetch_af2) to fetch_pdb to interact with structures in the AF DB via their UniProt ID

Related issues or pull requests

None

Pull Request Checklist

Added a note about the modification or contribution to the ./docs/sources/CHANGELOG.md file (if applicable)
Added appropriate unit test functions in the ./biopandas/*/tests directories (if applicable)
Modify documentation in the corresponding Jupyter Notebook under biopandas/docs/sources/ (if applicable)
Ran PYTHONPATH='.' pytest ./biopandas -sv and make sure that all unit tests pass (for small modifications, it might be sufficient to only run the specific test file, e.g., PYTHONPATH='.' pytest ./biopandas/classifier/tests/test_stacking_cv_classifier.py -sv)
Checked for style issues by running flake8 ./biopandas

pep8speaks · 2022-04-30T14:46:28Z

Hello @a-r-j! Thanks for updating this PR.

In the file biopandas/mmcif/pandas_mmcif.py:

Line 70:89: E501 line too long (113 > 88 characters)
Line 71:89: E501 line too long (123 > 88 characters)
Line 77:89: E501 line too long (104 > 88 characters)
Line 80:89: E501 line too long (116 > 88 characters)
Line 83:89: E501 line too long (125 > 88 characters)
Line 94:89: E501 line too long (107 > 88 characters)
Line 99:39: E203 whitespace before ':'
Line 100:89: E501 line too long (97 > 88 characters)
Line 101:41: E203 whitespace before ':'
Line 102:89: E501 line too long (101 > 88 characters)
Line 113:89: E501 line too long (118 > 88 characters)
Line 154:89: E501 line too long (102 > 88 characters)

In the file biopandas/mmcif/tests/test_read_mmcif.py:

Line 10:1: F401 'urllib.request.urlopen' imported but unused
Line 26:89: E501 line too long (99 > 88 characters)
Line 132:9: F841 local variable 'url' is assigned to but never used
Line 137:89: E501 line too long (95 > 88 characters)

In the file biopandas/pdb/pandas_pdb.py:

Line 118:5: E303 too many blank lines (2)
Line 118:89: E501 line too long (111 > 88 characters)
Line 119:89: E501 line too long (121 > 88 characters)
Line 125:89: E501 line too long (104 > 88 characters)
Line 128:89: E501 line too long (116 > 88 characters)
Line 131:89: E501 line too long (134 > 88 characters)
Line 142:89: E501 line too long (106 > 88 characters)
Line 145:9: E303 too many blank lines (2)
Line 148:39: E203 whitespace before ':'
Line 149:89: E501 line too long (97 > 88 characters)
Line 150:41: E203 whitespace before ':'
Line 151:89: E501 line too long (101 > 88 characters)
Line 162:89: E501 line too long (121 > 88 characters)
Line 346:89: E501 line too long (102 > 88 characters)

In the file biopandas/pdb/tests/test_read_pdb.py:

Line 9:1: F401 'urllib.error.URLError' imported but unused
Line 10:1: F401 'urllib.request.urlopen' imported but unused
Line 137:9: F841 local variable 'url' is assigned to but never used
Line 143:89: E501 line too long (93 > 88 characters)

Comment last updated at 2022-05-12 01:49:06 UTC

review-notebook-app · 2022-04-30T14:46:29Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

a-r-j · 2022-04-30T14:47:15Z

@rasbt Will add a note to the changelog but unsure how you want to proceed wrt naming convention.

biopandas/pdb/pandas_pdb.py

rasbt

This is really cool stuff! Thanks for this!

Btw. in addition to the other suggestions, one thing I couldn't annotate in the review because the files were empty, so I am commenting about this here: do we need the __init__.py files in the test folders? Just curious what they do? (I mean there is maybe an advantage, but could you maybe summarize it?)

biopandas/mmcif/pandas_mmcif.py

biopandas/pdb/pandas_pdb.py

biopandas/pdb/tests/test_read_pdb.py

a-r-j · 2022-04-30T21:29:13Z

I added the __init__s because tests would fail locally for me due to the overlap in the filenames for the mmcif and pdb testsuites. Adding the inits differentiates them in the namespace.

rasbt · 2022-04-30T21:30:00Z

Makes sense, thanks!

rasbt

Looks good!

rasbt · 2022-04-30T22:57:01Z

biopandas/mmcif/pandas_mmcif.py


 class PandasMmcif:
-    def __init__(self, use_auth: bool = True):
+    def __init__(self, use_auth: bool = True, af2_version: int = 2):


Would this be for specifying which alphafold database to use? I don't think we need this as an __init__ attribute though? Can't it be specified in fetch_mmcif(source="")?

E.g., something like

if source == 'alphafold2-v1': af2_version = 1 elif source == 'alphafold2-v2': af2_version = 2 else: ...

Yep, good call I think it belongs somewhere else. The param specifies which release of the database to retrieve the structure from. I don't anticipate it being changed much - the latest version is what I expect 99% of people would use - it was intended to reduce the maintenance burden in between new releases and updating the default.

rasbt · 2022-04-30T23:17:38Z

biopandas/mmcif/pandas_mmcif.py

        return self

-    def fetch_mmcif(self, pdb_code: Optional[str] = None, uniprot_id: Optional[str] = None, source: str = "pdb"):
+    def fetch_mmcif(self, pdb_code: Optional[str] = None, uniprot_id: Optional[str] = None, source: str = "pdb", af2_version: int = 2):


This would work, but i wonder if it wouldn't make more sense to make it part of the string, otherwise, we end up with a very specific argument that only makes sense if users select "alphafold2"?

I think setting it in the string is still problematic as it would require an update to biopandas to support each new release of the database before it can be used

Hm, I feel like this kind of complicated the user-interface too much. This is a value very specific to one particular "source" choice. Sure, we only have 2 source choices at the moment, but there could be more in the future (e.g., competitors to AF2, potentially AF3 etc.) and then it would become very messy for users to figure out which options go together.

I feel like it would be much simpler to support a fixed number of options for the sake of user-friendliness. If there is a v3 in the future, I don't think it would be much effort to update.

Sure, I've made the changes :)

Nice, thanks!

rasbt · 2022-05-12T00:12:30Z

Looks good to me know! The documentation currently doesn't fully execute because the part on multiple models is not here yet, but I can take care of that when it is merged into main. So the only thing left is the Changelog note, unless you have anything else you would like to add?
Great PR btw! Thanks a lot!

a-r-j · 2022-05-12T00:36:23Z

Nothing further from me, but I might pick up some loose threads from the open issues over the summer 😁 Thanks for all the help on the PRs - it's super fun working on this with you and I'm learning lots!

rasbt · 2022-05-12T01:49:56Z

Awesome! 🎉🙌.
I will actually make a new release tonight then so people can start using it 🤗

a-r-j added 2 commits April 30, 2022 15:42

add AF2 read method for pandaspdb

07d136f

add test data and update tutorial

ab34758

a-r-j commented Apr 30, 2022

View reviewed changes

biopandas/pdb/pandas_pdb.py Outdated Show resolved Hide resolved

a-r-j added 4 commits April 30, 2022 15:57

update tutorial notebook example numbering

ab4ad16

resolve merge conflicts

e757062

Merge branch 'rasbt-main' into af2

2b71854

add equivalent method for AF2

a87e81c

rasbt requested changes Apr 30, 2022

View reviewed changes

biopandas/mmcif/pandas_mmcif.py Outdated Show resolved Hide resolved

biopandas/pdb/pandas_pdb.py Outdated Show resolved Hide resolved

biopandas/pdb/tests/test_read_pdb.py Outdated Show resolved Hide resolved

remove py2 try/except for urllib imports

5000471

rasbt approved these changes Apr 30, 2022

View reviewed changes

a-r-j added 3 commits April 30, 2022 23:41

refactor AF2 fetch API

eb9d6ec

resolve merge

db14273

make af2 release version changeable at init

3149644

rasbt reviewed Apr 30, 2022

View reviewed changes

move af2 version to fetch_pdb method with default

c20d937

rasbt reviewed Apr 30, 2022

View reviewed changes

a-r-j and others added 3 commits May 1, 2022 16:10

update tutorial notebooks

eaf8b61

resolve merge conflict in jupyter notebook

189892b

Merge branch 'main' into af2

be6dd2f

update changelog and docs

6109d0b

rasbt merged commit dfac9fc into BioPandas:main May 12, 2022

Conversation

a-r-j commented Apr 30, 2022 • edited by rasbt Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues or pull requests

Pull Request Checklist

Uh oh!

pep8speaks commented Apr 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated at 2022-05-12 01:49:06 UTC

Uh oh!

review-notebook-app bot commented Apr 30, 2022

Uh oh!

a-r-j commented Apr 30, 2022

Uh oh!

Uh oh!

rasbt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

a-r-j commented Apr 30, 2022

Uh oh!

rasbt commented Apr 30, 2022

Uh oh!

rasbt left a comment

Choose a reason for hiding this comment

Uh oh!

rasbt Apr 30, 2022

Choose a reason for hiding this comment

Uh oh!

a-r-j Apr 30, 2022

Choose a reason for hiding this comment

Uh oh!

rasbt Apr 30, 2022

Choose a reason for hiding this comment

Uh oh!

a-r-j Apr 30, 2022

Choose a reason for hiding this comment

Uh oh!

rasbt May 1, 2022

Choose a reason for hiding this comment

Uh oh!

a-r-j May 1, 2022

Choose a reason for hiding this comment

Uh oh!

rasbt May 11, 2022

Choose a reason for hiding this comment

Uh oh!

rasbt commented May 12, 2022

Uh oh!

a-r-j commented May 12, 2022

Uh oh!

rasbt commented May 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

a-r-j commented Apr 30, 2022 •

edited by rasbt

Loading

pep8speaks commented Apr 30, 2022 •

edited

Loading